Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skipping files and lines. #218

Closed
wants to merge 5 commits into from

Conversation

zundertj
Copy link
Contributor

@zundertj zundertj commented Mar 10, 2019

Work in progress; just setting this up to get your input.

Adresses #53 & #44; supersedes #55.

Main design issues/questions/todo's:

  • should a .coverage.yml file only affect files in the folder, or also sub folders? I guess most packages have just a single src folder, but having to specify the exclusion pattern for each and every folder in case there are multiple seems tedious to me, in particular for the line pattern. Alternatively, a much simpler solution would be to simply have a toggle or default regex in the API, removing the need for the .coverage.yml file for the line patterns altogether. That does not allow one to completely exclude files tough.
  • The current implementation fails as amend_coverage_from_src modifies the results afterwards. There is more code change coming for that function (PR214), so I did not want to touch that function right now. To make this work, it seems that we have to reorganize these two functions. I'm happy to do that once the dust is settled, if you agree that is the right approach.
  • add whatever implementation we end up with to the README.

Fixes #44
Fixes #53
Closes #55

@vtjnash
Copy link
Member

vtjnash commented May 15, 2019

The current implementation fails as amend_coverage_from_src modifies the results afterwards

Due to this, I think it would be better for this implementation to live inside that function entirely. After all, this is exactly ‘amending coverage from looking at the source code’.

@codecov-io
Copy link

codecov-io commented Jun 23, 2019

Codecov Report

Merging #218 into master will decrease coverage by 0.51%.
The diff coverage is 93.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #218      +/-   ##
==========================================
- Coverage   91.36%   90.85%   -0.52%     
==========================================
  Files           6        6              
  Lines         336      361      +25     
==========================================
+ Hits          307      328      +21     
- Misses         29       33       +4
Impacted Files Coverage Δ
src/Coverage.jl 96.21% <93.93%> (-1.92%) ⬇️
src/lcov.jl 94.11% <0%> (-1.97%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ecba42...4599a20. Read the comment docs.

@zundertj
Copy link
Contributor Author

Resolved conflicts and moved the line-by-line functionality to amend_coverage_from_src. The only question left is whether we should apply config files also to sub folders, at the moment it does not.

src/Coverage.jl Outdated Show resolved Hide resolved
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some remarks...

could you also perhaps git rebase master?

src/Coverage.jl Outdated Show resolved Hide resolved
src/Coverage.jl Outdated Show resolved Hide resolved
src/Coverage.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
src/Coverage.jl Show resolved Hide resolved
@zundertj zundertj force-pushed the skip_files_and_lines branch 2 times, most recently from 7c533c7 to 3fcb26c Compare July 14, 2019 17:45
@zundertj zundertj force-pushed the skip_files_and_lines branch from bc66bff to 7d18c7a Compare July 14, 2019 19:14
src/Coverage.jl Outdated Show resolved Hide resolved
@zundertj zundertj changed the title WIP: Skipping files and lines. Skipping files and lines. Jul 14, 2019
@zundertj
Copy link
Contributor Author

So I have all the test working now locally, but both Travis and AppVeyor keep failing. If anyone has any pointers, that would be much appreciated.

@DilumAluthge
Copy link
Member

@zundertj Why did you delete the file test/data/Coverage.jl.cov? Don’t you need to have that file inside the test/data/no_exclusions directory?

@DilumAluthge
Copy link
Member

DilumAluthge commented Sep 19, 2019

@zundertj On an unrelated note, could you edit the original post to have the following content exactly? That way, when this PR is merged, GitHub will automatically close the mentioned issues and pull requests. (But you have to use those specific words like Fixes and Closes in order for GitHub to understand you.)

Fixes #44
Fixes #53
Closes #55

@DilumAluthge
Copy link
Member

Now that the core functionality has been factored out to CoverageTools.jl, this PR will need to be made to the https://github.com/JuliaCI/CoverageTools.jl repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have an option to mark lines as "ok that this isn't covered" Excluding some files from coverage
5 participants